Skip to content

Improve libformat_parser FFI #4074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 18, 2025
Merged

Conversation

powerboat9
Copy link
Collaborator

This should remove a use-after-free as well as simplify the FFI interface.

I think this should all be correct -- at the very least, it should be more correct

@powerboat9 powerboat9 force-pushed the fix-rust-issues branch 3 times, most recently from 1a3e8a4 to acc2cfd Compare August 13, 2025 01:29
This should remove a use-after-free as well as simplify the FFI
interface.

gcc/rust/ChangeLog:

	* ast/rust-fmt.cc (Pieces::collect): Handle changes to ffi
	interface.
	(Pieces::~Pieces): Remove function definition.
	(Pieces::Pieces): Likewise.
	(Pieces::operator=): Likewise.
	* ast/rust-fmt.h: Include "optional.h".
	(rust_ffi_alloc): New extern "C" function declaration.
	(rust_ffi_dealloc): Likewise.
	(class FFIVec): New class.
	(class FFIOpt): Likewise.
	(RustHamster::RustHamster): New constructor accepting const
	std::string reference.
	(struct FormatSpec): Use FFIOpt.
	(struct PieceSlice): Remove struct.
	(struct RustString): Likewise.
	(struct FormatArgsHandle): Likewise.
	(collect_pieces): Change function signature.
	(clone_pieces): Likewise.
	(destroy_pieces): Remove extern "C" function declaration.
	(Pieces::~Pieces): Remove function declaration.
	(Pieces::operator=): Likewise.
	(Pieces::get_pieces): Handle changes to class fields.
	(Pieces::Pieces): Remove copy and move constructor declarations,
	adjust signature of remaining constructor declaration.
	(Pieces::pieces_vector): Remove member variable.
	(Pieces::handle): Likewise.
	(Pieces::data): Add member variable.
	* expand/rust-macro-builtins-asm.cc (expand_inline_asm_strings):
	Use references to avoid copying.

libgrust/ChangeLog:

	* libformat_parser/src/lib.rs (struct FFIVec): New.
	(trait StringLeakExt): Remove.
	(struct FFIOpt): New.
	(trait IntoFFI): Adjust implementation for Option.
	(struct RustHamster): Add lifetime and adjust conversion to and
	from &str.
	(enum Piece): Adjust definition to handle changes to
	RustHamster.
	(struct Argument): Likewise.
	(struct FormatSpec): Use FFIOpt and RustHamster.
	(enum Position): Use RustHamster.
	(enum Count): Likewise.
	(struct PieceSlice): Replace with...
	(typedef PieceVec): ...this.
	(struct RustString): Remove.
	(struct FormatArgsHandle): Likewise.
	(fn collect_pieces): Adjust signature, greatly simplifying
	implementation.
	(fn clone_pieces): Likewise.
	(fn destroy_pieces): Remove.
	(trait LayoutExt): New.
	(fn rust_ffi_alloc): New.
	(fn rust_ffi_dealloc): New.

Signed-off-by: Owen Avery <[email protected]>
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like it, thank you for spending some time on this. I checked and it does not fix our ASAN issues with Rust 1.49 but this is not important. I would say one nit which is I'd like to see all these types in their own FFI namespace instead of being called FFI* but this is super unimportant and can be done later down the line. Good work!

@CohenArthur CohenArthur added this pull request to the merge queue Aug 18, 2025
Merged via the queue into Rust-GCC:master with commit 2d376a2 Aug 18, 2025
13 checks passed
@powerboat9 powerboat9 deleted the fix-rust-issues branch August 18, 2025 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants